Skip to content

Conversation

AlanCoding
Copy link
Member

@AlanCoding AlanCoding commented Oct 2, 2025

Description

Looking to get DAB CI passing with Django 5 & 4 for the compatibility phase.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Test update
  • Refactoring (no functional changes)
  • Development environment change
  • Configuration change

Note

Adds Django 5.2 support (while keeping Django 4.x), updates requirements, adapts prefetch API, expands CI/tox matrices, adds tests, and includes a help_text-only migration.

  • Django 5.2 compatibility:
    • Update ansible_base/resource_registry/fields.py to use get_prefetch_querysets(...) with a Django 4 compatibility wrapper get_prefetch_queryset(...).
    • Broaden Django requirement to >=4.2.21,<6.0; pin dev/lock to django==5.2.7.
  • CI/Build:
    • Expand GitHub Actions matrix and tox envs with py310-django4, py311-django4, py312-django4.
    • Update tox labels to include new Django 4 lanes.
    • requirements/updater.sh: add safety check to require running from requirements/ dir.
  • Database migration:
    • Add dab_rbac migration 0009_objectrole_help_text_change.py to adjust ObjectRole.users and .teams help_text.
  • Tests:
    • Extend resource registry tests to cover prefetch_related across organization__resource chains and to resource__content_type__resource_type.

Written by Cursor Bugbot for commit 367c49f. This will update automatically on new commits. Configure here.

@AlanCoding AlanCoding changed the title Try to run checks with Django 5.2 AAP-51350 Try to run checks with Django 5.2 Oct 2, 2025
@AlanCoding
Copy link
Member Author

Documenting:

FAILED test_app/tests/resource_registry/models/test_resource_field.py::test_resource_field_prefetch_related - AttributeError: 'NoneType' object has no attribute 'name'
FAILED test_app/tests/rbac/remote/test_service_api.py::test_list_role_user_assignments - AssertionError: assert None == 'd81556af-c82e-4659-bde4-b0905f1a696b'
 +  where 'd81556af-c82e-4659-bde4-b0905f1a696b' = str(UUID('d81556af-c82e-4659-bde4-b0905f1a696b'))
 +    where UUID('d81556af-c82e-4659-bde4-b0905f1a696b') = <Resource: Resource object (974)>.ansible_id
 +      where <Resource: Resource object (974)> = <User: rando>.resource
FAILED test_app/tests/resource_registry/test_resources_api_rest_client.py::test_sync_org_assignment - AssertionError: assert 'f7f63489-91a5-4a9a-8eaa-ba7e409e1375' == None
 +  where 'f7f63489-91a5-4a9a-8eaa-ba7e409e1375' = str(UUID('f7f63489-91a5-4a9a-8eaa-ba7e409e1375'))
 +    where UUID('f7f63489-91a5-4a9a-8eaa-ba7e409e1375') = <Resource: Resource object (1509)>.ansible_id
 +      where <Resource: Resource object (1509)> = <User: _system>.resource
 +        where <User: _system> = RoleUserAssignment(pk=122).created_by
FAILED test_app/tests/resource_registry/test_resources_api.py::test_resource_summary_fields - AssertionError: assert 'resource' in {'modified_by': {'id': 878, 'username': '_system', 'first_name': '', 'last_name': ''}, 'created_by': {'id': 878, 'username': '_system', 'first_name': '', 'last_name': ''}}
FAILED test_app/tests/resource_registry/test_resources_api_rest_client.py::test_sync_obj_assignment - AssertionError: assert '1dc0fc62-4480-4198-9d7b-f9f5acee18c9' == None
 +  where '1dc0fc62-4480-4198-9d7b-f9f5acee18c9' = str(UUID('1dc0fc62-4480-4198-9d7b-f9f5acee18c9'))
 +    where UUID('1dc0fc62-4480-4198-9d7b-f9f5acee18c9') = <Resource: Resource object (1515)>.ansible_id
 +      where <Resource: Resource object (1515)> = <User: _system>.resource
 +        where <User: _system> = RoleUserAssignment(pk=123).created_by
FAILED test_app/tests/resource_registry/test_resources_api_rest_client.py::test_list_user_assignments - AssertionError: User assignment not found in list results
assert False
FAILED test_app/tests/resource_registry/test_resources_api_rest_client.py::test_list_team_assignments - AssertionError: Team assignment not found in list results
assert False
FAILED test_app/tests/rbac/remote/test_service_api.py::TestCreatedByAnsibleIdAllowNull::test_list_assignments_shows_created_by_when_present - AssertionError: assert None == 'de23093f-5d72-4de3-bced-660c8b0b1817'
 +  where 'de23093f-5d72-4de3-bced-660c8b0b1817' = str(UUID('de23093f-5d72-4de3-bced-660c8b0b1817'))
 +    where UUID('de23093f-5d72-4de3-bced-660c8b0b1817') = <Resource: Resource object (1105)>.ansible_id
 +      where <Resource: Resource object (1105)> = <User: assignment-creator>.resource
FAILED test_app/tests/rbac/remote/test_service_api.py::TestCreatedByAnsibleIdAllowNull::test_list_assignments_shows_null_created_by_when_null - assert None is not None
===== 9 failed, 2619 passed, 13 xfailed, 62 warnings in 559.17s (0:09:19) ======

@AlanCoding
Copy link
Member Author

Link #213

@AlanCoding AlanCoding marked this pull request as ready for review October 6, 2025 14:23
cursor[bot]

This comment was marked as outdated.

@AlanCoding
Copy link
Member Author

@ttuffin asking for your advisement on SonarCloud coverage data here. This reports that the new method isn't covered. This is somewhat obvious, because it is added for compatibility, and whatever the existing config is doing seems to only get coverage from one run. But clearly different runs in the matrix will cover different lines in code - otherwise why would we have a matrix?!

This is where a coverage requirement becomes very murky, as it gets highly highly configuration dependent. Do you want coverage to be gathered from all runs and combined? We have a clear preference, which is that we only "care" about Django 5, as the compatibility stuff is temporary, but cannot be avoided. Getting coverage for version-specific stuff is extra work and doesn't necessarily provide any value.

cursor[bot]

This comment was marked as outdated.

@AlanCoding AlanCoding changed the title AAP-51350 Try to run checks with Django 5.2 AAP-51350 Support Django 5.2 as well as Django 4.y Oct 6, 2025
cursor[bot]

This comment was marked as outdated.

@AlanCoding
Copy link
Member Author

Tests I added meaningfully covered the non-trivial code related to processing of multiple querysets passed in. This seems pretty solid now.

@ttuffin
Copy link

ttuffin commented Oct 7, 2025

@ttuffin asking for your advisement on SonarCloud coverage data here. This reports that the new method isn't covered. This is somewhat obvious, because it is added for compatibility, and whatever the existing config is doing seems to only get coverage from one run. But clearly different runs in the matrix will cover different lines in code - otherwise why would we have a matrix?!

This is where a coverage requirement becomes very murky, as it gets highly highly configuration dependent. Do you want coverage to be gathered from all runs and combined? We have a clear preference, which is that we only "care" about Django 5, as the compatibility stuff is temporary, but cannot be avoided. Getting coverage for version-specific stuff is extra work and doesn't necessarily provide any value.

@AlanCoding one potential solution could be to update the CI to generate coverage reports for each run, aggregate them and then send that aggregated report to SonarCloud. I am not sure what the long-term plan is for supporting Django 4 in dab, but I imagine we may still need add potential bug/CVE fixes in the future? In which case having coverage for Django 4 related code would be advantageous. Aggregated coverage reports is not something we covered in the initial proposal - perhaps it would be good to add some guidance around that.

All that said, I am happy to leave something like this to the discretion of the PDT's since you know your codebase best. In this particular case, SonarCloud appears to be getting caught up on code that you consider to be temporary for backwards compatibility purposes only, in which case I don't see a problem in excluding it with # NOSONAR.

HTH. If you think we should have better guidance about this from the SIG I'd be happy to raise it for discussion.

@AlanCoding
Copy link
Member Author

I am not sure what the long-term plan is for supporting Django 4 in dab, but I imagine we may still need add potential bug/CVE fixes in the future?

No it will not. We only need this temporarily in a mid-release sense. We need Django 4 support in this branch, only as a matter of branch management. It is a big undertaking to get users of DAB to upgrade, and may take a month or more.

I'll try to add NOSONAR

Copy link

github-actions bot commented Oct 7, 2025

DVCS PR Check Results:

PR appears valid (JIRA key(s) found)

Copy link

sonarqubecloud bot commented Oct 7, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
4 New issues
3 Security Hotspots
66.7% Coverage on New Code (required ≥ 80%)
E Security Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants